Skip to content

feat: flush hook extension point#8145

Draft
sunng87 wants to merge 6 commits into
mainfrom
feat/flush-hook-extension-point
Draft

feat: flush hook extension point#8145
sunng87 wants to merge 6 commits into
mainfrom
feat/flush-hook-extension-point

Conversation

@sunng87
Copy link
Copy Markdown
Member

@sunng87 sunng87 commented May 21, 2026

I hereby agree to the terms of the GreptimeDB CLA.

Refer to a related PR or issue link (optional)

What's changed and what's your intention?

This patch adds extension points and plugin resgistration for flush/compaction.

PR Checklist

Please convert it to a draft if some of the following conditions are not met.

  • I have written the necessary rustdoc comments.
  • I have added the necessary unit tests and integration tests.
  • This PR requires documentation updates.
  • API changes are backward compatible.
  • Schema or data changes are backward compatible.

@github-actions github-actions Bot added size/M docs-not-required This change does not impact docs. labels May 21, 2026
gemini-code-assist[bot]
gemini-code-assist Bot reviewed May 21, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces a FlushHook extension point for the mito2 engine, allowing plugins to receive notifications when SST files are written or the manifest is updated during flush and compaction operations. The implementation includes a new PkCollectingIter to gather unique primary keys during these processes. Feedback from the review highlights a critical bug in the primary key column indexing logic and a serialization issue where hook-related data is skipped in MergeOutput, which would break remote compaction. Additionally, there are suggestions to optimize performance by using references instead of cloning FileMeta and a warning regarding potential memory exhaustion when collecting a large volume of primary keys.

Comment thread src/mito2/src/compaction/compactor.rs Outdated
Comment thread src/mito2/src/flush.rs Outdated
Comment thread src/mito2/src/compaction/task.rs Outdated
Comment thread src/mito2/src/engine/flush_hook.rs Outdated
Comment thread src/mito2/src/flush.rs Outdated
Comment thread src/mito2/src/flush.rs Outdated
@sunng87 sunng87 force-pushed the feat/flush-hook-extension-point branch from 9a15670 to 9ac92e5 Compare May 21, 2026 16:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@evenyag evenyag Awaiting requested review from evenyag evenyag will be requested when the pull request is marked ready for review evenyag is a code owner

@v0y4g3r v0y4g3r Awaiting requested review from v0y4g3r v0y4g3r will be requested when the pull request is marked ready for review v0y4g3r is a code owner

@waynexia waynexia Awaiting requested review from waynexia waynexia will be requested when the pull request is marked ready for review waynexia is a code owner

1 more reviewer

@gemini-code-assist gemini-code-assist[bot] gemini-code-assist[bot] left review comments

Reviewers whose approvals may not affect merge requirements

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

docs-not-required This change does not impact docs. size/M

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@sunng87